feat(conversion): pre-expiry trial reminder email (R1)#939
Conversation
|
@claude CI has FAILED. Use your Read tool to read the local file Context
Failed Jobs
|
|
Claude finished @FagnerMartinsBrack's task in 8m 50s —— View job CI Failure Fix — Attempt 1 of 5
Root cause: pre-existing failure on
|
| Check | Result |
|---|---|
main push-CI for 5b993302 (run 28744076544) |
❌ byte-identical failure (same 2 tests, 145→153px, 990px, ratio 0.12) |
#936's own PR check (web-tests, commit c32904bf) |
❌ was already failure — #936 merged into main with a red check |
| This PR's rendering changes | none — full diff is trial-reminder only; the one "bundle"-named file (web-test-harness/src/bundle.types.ts) is types-only |
| Baseline snapshot history | committed once, by 5b993302; never matched the CI renderer |
The capsule is 8px wider because the fixed, timezone-pinned label "26 Mar '26, 14:32" occupies more horizontal space on the CI chromium-linux renderer than on whatever machine generated the committed 145×54 baseline. .crawl-bookmark__time sets no explicit font-family, so this is a classic web-font-fallback / off-platform baseline mismatch.
Why I did not push a change here
There is no in-scope fix for this PR. Correcting #936's binary snapshot baseline from inside an unrelated trial-reminder PR would:
- violate "do not make unrelated changes" (the PR touches no rendering code, and the snapshot files aren't even on this branch — they arrive via the merge);
- risk masking a real bug — if a font genuinely fails to load in the reader, the correct fix is to make it load, not to bless the
153pxfallback as the new baseline (I can't verify which rendering is intended from here); - collide with the proper fix once it lands on
main.
main is currently red for every PR, so no amount of re-running this fixer on #939 will make it green — the fix must land on main.
Recommended fix (on main, in #936's context)
Reproduce on the CI runner, then either:
- If a web font is failing to load (most likely) → fix the font loading for the crawl bookmark; or
- If the baseline was captured off-platform → regenerate both baselines with
playwright --update-snapshotson the CI-equivalent chromium-linux runner.
Then rebase this branch onto the fixed main — #939 goes green with zero changes here. Fix this →
No commit pushed: the failure originates entirely in main (#936) and this PR contains no rendering changes. Pushing an unrelated snapshot edit would mix concerns and could mask a real font-loading regression.
· failing PR CI · branch conversion/trial-reminder-email
|
@claude Review this PR. Use your Read tool to read the local file Context
|
|
Claude finished @FagnerMartinsBrack's task in 6m 20s —— View job Code Review — PR #939 (pre-expiry trial reminder email)
Review complete — 0 high / 0 medium / 2 low. The full review is emitted to the code-review automation, which posts it as the review comment (kept out of this tracking comment to avoid a duplicate). TL;DR: Strong, additive PR. Perfect create/delete parity across all schedule sites, backward-compatible optional |
High Priority Issues (Must Fix)None found Medium Priority IssuesNone found Low Priority Suggestions1. Singular subject-verb disagreement in the saved-articles clause
"the 1 article … stay" reads as a subject-verb disagreement (should be "stays"). It's a deliberate choice — 2. "At most one reminder" is a read-then-write guard, not an atomic conditional writeThe single-send property in This mirrors the existing |
|
@claude Resolve conflicts. Use your Read tool to read the local file Context
|
|
Claude finished @FagnerMartinsBrack's task in 13m 40s —— View job Conflicts resolved ✅Rebased
What conflictedOne file:
Resolution: kept both — main's unconditional call structure and the feature's new The other overlapping files — History cleanupThe pre-existing merge commit ( Verification
|
Add a founder-voice reminder email that fires two days before trialEndsAt
for card-free trials, reusing the existing send-trial-feedback-email Lambda
and EventBridge Scheduler pipeline (no new Lambda, queue, event rule, or IAM).
SendTrialFeedbackEmailCommand gains an optional kind ("feedback"|"reminder",
absent=feedback for backward compatibility). A deterministic one-shot
trial-reminder-<userId> targets the same DetailType and carries
Input {userId, kind:"reminder"}. Schedules are created at all three
trial-signup sites (email/Google/Apple) and recreated on /account
reactivation when >2d remain; deleted at checkout success and the
cancel-subscription trialing branch. Idempotence via a new
trialReminderEmailSentAt row flag; the reminder path re-checks
status==="trialing" with a future trialEndsAt before sending. CTA links to
/account with utm_source=trial-reminder for attribution. Only infra delta is
APP_ORIGIN on the send-trial-feedback-email Lambda.
Claude-Session: https://claude.ai/code/session_01En2uUiiwQoRQKWenCnkPi9
da96a30 to
f1227b0
Compare
|
@claude Review this PR. Use your Read tool to read the local file Context
|
|
Claude finished @FagnerMartinsBrack's task in 6m 2s —— View job Code Review — PR #939 (pre-expiry trial reminder email)
Re-reviewed the rebased single-commit version ( TL;DR: Strong, additive PR. Verified independently: clean create/delete schedule parity across all four create sites (email/Google/Apple signup + guarded reactivation) and both delete sites (checkout success + cancel |
High Priority Issues (Must Fix)None found Medium Priority IssuesNone found Low Priority Suggestions1. Singular subject-verb disagreement in the saved-articles clause
"the 1 article … stay" is a subject-verb disagreement (should be "stays"). It's deliberate — 2. "At most one reminder" is a read-then-write guard, not an atomic conditional writeThe single-send property in This mirrors the existing |
…on-app-store-rasc6i Reconcile with #939 (pre-expiry trial reminder email), which extended the same subscription-providers store this PR extended for account-deletion scrubbing. Kept both sides' additions across the write provider, its unit test, and the in-memory fixture + test: - markTrialReminderEmailSent (#939) and deleteSubscription (this PR) - split the interleaved describe/it blocks back into complete blocks - corrected the auto-merged "seven"->"eight mutations" doc comment Co-authored-by: Fayner Brack <FagnerMartinsBrack@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The #939 merge added a fourth per-user EventBridge schedule (trial-reminder-<userId>), created for every trialing user at signup/trial-start, but the delete-account worker still deleted only three (trial-end, deferred-cancellation, trial-feedback). An account deleted mid-trial therefore left an orphaned, user-keyed reminder schedule live at AWS until its natural fire time — the exact kind of user-owned remnant this compliance feature exists to erase — and the handler's own "all three" comment misstated the count. Wire deleteTrialReminderSchedule into the worker alongside the other three deletes (it is ResourceNotFound-idempotent, so a redrive converges), correct the comment to "all four", and cover it in the trialing and founding-member branch tests plus the raw-email redrive test (which exercises the schedule delete on both the failed first delivery and the retry). No IAM change: the worker's scheduler:DeleteSchedule grant is a group-level wildcard that already permits the reminder schedule name. The sibling cancel-subscription worker already deletes this same schedule for a trialing cancel. Co-authored-by: Fayner Brack <FagnerMartinsBrack@users.noreply.github.com>
feat(conversion): pre-expiry trial reminder email (R1)
Hypothesis
Card-free trials expire silently: users get zero contact between signup and the moment their account flips read-only, so the trial ends by default rather than by decision. A founder-voice email 2 days before
trialEndsAt, with a direct subscribe CTA, converts some of that silent expiry into deliberate subscribe decisions (and honest churn signal on replies).Evidence (30d, Jun 4 – Jul 4 2026)
charge_failed reason=no_card_on_file→ auto-cancelled ~60 min later. Trial→paid = 0%.What this changes
SendTrialFeedbackEmailCommandgains optionalkind: "feedback" | "reminder"(absent = feedback → fully backward compatible with in-flight schedules).trial-reminder-<userId>(same target DetailType,Input {userId, kind:"reminder"},ActionAfterCompletion: DELETE) created at all three trial-signup sites (email, Google, Apple) attrialEndsAt − 2d, and recreated on trial reactivation when >2d remain.deleteTrialEndSchedulecall: checkout success and the cancel-subscriptiontrialingbranch.send-trial-feedback-emailLambda branches onkind. Reminder path re-checksstatus === "trialing"&&trialEndsAtin the future, and stamps a newtrialReminderEmailSentAtrow flag (markTrialReminderEmailSent) so at-least-once delivery can't double-send.trial-reminder-email), CTA →/account?utm_source=trial-reminder&utm_medium=email&utm_campaign=trial-preexpiry.APP_ORIGINon the send-trial-feedback-email Lambda. No new Lambda, no new queue, no new event rule, no IAM change (the scheduler-manage policy already wildcards the schedule group).Estimated result
~28 reminder emails/month reaching 100% of expiring card-free trials. At even 1-in-10 recipients clicking through and the observed 2/4 subscribe-click→checkout-success rate, this is the difference between 0%% and a measurable trial→paid baseline; secondarily, replies give pre-churn feedback while users are still active (the post-churn email pulls 0-save ghosts).
How to measure
Reminder sends (CloudWatch Logs Insights on
/aws/lambda/…send-trial-feedback-emaillog group):Reminder noops (stale schedules, races): filter
reminder:for the noop reasons.Arrivals: hutch analytics pageviews where the query string carries
utm_source=trial-reminder(click-attribution middleware already persists utm_source/medium/campaign), joined against subsequentsubscribeclicks and/auth/checkout/successhits. Success criterion after 30d: reminders_sent ≈ trials expiring, >0 checkout successes attributedtrial-reminder.Merge note
This PR adds no analytics events, streams, or log groups, so
analytics-dashboard.test.ts(toHaveLength(24)) is untouched. Sibling PRs #916–#918 may bump that widget count; if one lands first, a rebase here is conflict-free — if this lands first, they re-count from whatever main asserts. Resolve by keeping the assertion equal to the actual widget array length on the merged main, never by deleting widgets.https://claude.ai/code/session_01En2uUiiwQoRQKWenCnkPi9